Skip to content

Treat warnings as errors#294

Open
danth wants to merge 1 commit into
eclipse-score:mainfrom
etas-contrib:reject-warnings
Open

Treat warnings as errors#294
danth wants to merge 1 commit into
eclipse-score:mainfrom
etas-contrib:reject-warnings

Conversation

@danth

@danth danth commented Jul 8, 2026

Copy link
Copy Markdown
Member

This ensures that warnings are not ignored, by causing the build to fail.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.6.0) and connecting to it...
INFO: Invocation ID: 8a0437bf-8cac-46f5-840a-186cef32a673
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (38 packages loaded, 10 targets configured)

Analyzing: target //:license-check (78 packages loaded, 10 targets configured)

Analyzing: target //:license-check (144 packages loaded, 2767 targets configured)

Analyzing: target //:license-check (152 packages loaded, 5989 targets configured)

Analyzing: target //:license-check (160 packages loaded, 8220 targets configured)

Analyzing: target //:license-check (163 packages loaded, 8263 targets configured)

Analyzing: target //:license-check (164 packages loaded, 8271 targets configured)

Analyzing: target //:license-check (164 packages loaded, 8271 targets configured)

Analyzing: target //:license-check (164 packages loaded, 8271 targets configured)

Analyzing: target //:license-check (167 packages loaded, 10159 targets configured)

Analyzing: target //:license-check (169 packages loaded, 10409 targets configured)

INFO: Analyzed target //:license-check (169 packages loaded, 10409 targets configured).
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 24.613s, Critical Path: 2.76s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@danth

danth commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

The existing warnings should be fixed by #293

@danth danth force-pushed the reject-warnings branch from 1714d3c to ed3b91f Compare July 10, 2026 09:04
@danth danth temporarily deployed to workflow-approval July 10, 2026 09:04 — with GitHub Actions Inactive
@danth danth temporarily deployed to workflow-approval July 10, 2026 09:04 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

The created documentation from the pull request is available at: docu-html

@NicolasFussberger

NicolasFussberger commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

I wonder if we should add further warnings for compilation, e.g. like COM does https://github.com/eclipse-score/communication/blob/570e34715b2d3156d2ae2cc4686be29a8c92f3ef/MODULE.bazel#L97
Baselibs does similar things.
This would give some additional benefit to treating warnings as errors compared to now when we do not have many warnings active.

But we should also avoid that users will inherit those compiler flags from our targets.
Currently I am not sure what is the behavior.

@danth danth force-pushed the reject-warnings branch from 9ae74d6 to d54db7b Compare July 10, 2026 13:49
@danth danth requested a deployment to workflow-approval July 10, 2026 13:49 — with GitHub Actions Waiting
@danth danth requested a deployment to workflow-approval July 10, 2026 13:49 — with GitHub Actions Waiting
@danth danth force-pushed the reject-warnings branch from d54db7b to add044a Compare July 10, 2026 14:34
@danth danth temporarily deployed to workflow-approval July 10, 2026 14:34 — with GitHub Actions Inactive
@danth danth temporarily deployed to workflow-approval July 10, 2026 14:34 — with GitHub Actions Inactive
@danth

danth commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

I wonder if we should add further warnings for compilation

I've enabled -Wall and -Wextra, except for a couple of warnings present in baselibs.

There is also -Wpedantic, but it seems like there would be a lot of changes needed to build successfully with that.

But we should also avoid that users will inherit those compiler flags from our targets. Currently I am not sure what is the behavior.

Flags set in .bazelrc are only applied when Bazel runs inside this repository, if that is what you mean: https://bazel.build/run/bazelrc#bazelrc-file-locations

@NicolasFussberger

Copy link
Copy Markdown
Contributor

QNX build fails with the following error:

external/googletest+/googletest/src/gtest-death-test.cc:1283:34: error: missing initializer for member 'inheritance::pgroup' [-Werror=missing-field-initializers]
 1283 |   struct inheritance inherit = {0};
      |

I wonder if this will become an issue that new compiler warning in any dependency may break our build in the future. Is there a possibility to limit the warnings only to our own code?

@NicolasFussberger NicolasFussberger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

@danth danth force-pushed the reject-warnings branch from 12e3467 to 76f3218 Compare July 13, 2026 10:42
@danth danth requested a deployment to workflow-approval July 13, 2026 10:42 — with GitHub Actions Waiting
@danth danth requested a deployment to workflow-approval July 13, 2026 10:42 — with GitHub Actions Waiting
@danth danth marked this pull request as draft July 13, 2026 10:49
@danth danth force-pushed the reject-warnings branch from 76f3218 to 7ff46d8 Compare July 13, 2026 12:04
@danth danth requested a deployment to workflow-approval July 13, 2026 12:04 — with GitHub Actions Waiting
@danth danth requested a deployment to workflow-approval July 13, 2026 12:04 — with GitHub Actions Waiting
@danth danth marked this pull request as ready for review July 13, 2026 12:15
@danth

danth commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

Just pushed a new implementation which should not apply the flags to our dependencies, nor to anything depending on us.

However it means we have to call a different rule which wraps the standard cc_library / cc_binary to add our options.

@danth danth requested a review from NicolasFussberger July 13, 2026 12:15
@danth danth force-pushed the reject-warnings branch from 7ff46d8 to dce0d3d Compare July 13, 2026 12:17
@danth danth requested a deployment to workflow-approval July 13, 2026 12:18 — with GitHub Actions Waiting
@danth danth requested a deployment to workflow-approval July 13, 2026 12:18 — with GitHub Actions Waiting
@danth danth force-pushed the reject-warnings branch from dce0d3d to 3c7194c Compare July 13, 2026 12:20
@danth danth requested a deployment to workflow-approval July 13, 2026 12:20 — with GitHub Actions Waiting
@danth danth requested a deployment to workflow-approval July 13, 2026 12:20 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants